[lldb] Introduce ScriptedFrameProvider for real threads #161870
Conversation
medismailben
changed the title
|
@llvm/pr-subscribers-lldb Author: Med Ismail Bennani (medismailben) ChangesPatch is 27.35 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/161870.diff 19 Files Affected:
diff --git a/lldb/include/lldb/API/SBThread.h b/lldb/include/lldb/API/SBThread.h
index e9fe5858d125e..7b91228528bc7 100644
--- a/lldb/include/lldb/API/SBThread.h
+++ b/lldb/include/lldb/API/SBThread.h
@@ -229,6 +229,8 @@ class LLDB_API SBThread {
SBValue GetSiginfo();
+ void RegisterFrameProvider(const char *class_name, SBStructuredData &args_data);
+
private:
friend class SBBreakpoint;
friend class SBBreakpointLocation;
diff --git a/lldb/include/lldb/Interpreter/Interfaces/ScriptedFrameProviderInterface.h b/lldb/include/lldb/Interpreter/Interfaces/ScriptedFrameProviderInterface.h
new file mode 100644
index 0000000000000..7618d5e15d563
--- /dev/null
+++ b/lldb/include/lldb/Interpreter/Interfaces/ScriptedFrameProviderInterface.h
@@ -0,0 +1,27 @@
+//===-- ScriptedFrameProviderInterface.h ------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_INTERPRETER_INTERFACES_SCRIPTEDFRAMEPROVIDERINTERFACE_H
+#define LLDB_INTERPRETER_INTERFACES_SCRIPTEDFRAMEPROVIDERINTERFACE_H
+
+#include "lldb/lldb-private.h"
+
+#include "ScriptedInterface.h"
+
+namespace lldb_private {
+class ScriptedFrameProviderInterface : public ScriptedInterface {
+public:
+ virtual llvm::Expected<StructuredData::GenericSP>
+ CreatePluginObject(llvm::StringRef class_name, lldb::ThreadSP thread_sp,
+ StructuredData::DictionarySP args_sp) = 0;
+
+ virtual StructuredData::ArraySP GetStackFrames() { return {}; }
+};
+} // namespace lldb_private
+
+#endif // LLDB_INTERPRETER_INTERFACES_SCRIPTEDFRAMEPROVIDERINTERFACE_H
diff --git a/lldb/include/lldb/Interpreter/ScriptInterpreter.h b/lldb/include/lldb/Interpreter/ScriptInterpreter.h
index 024bbc90a9a39..76ade002089bb 100644
--- a/lldb/include/lldb/Interpreter/ScriptInterpreter.h
+++ b/lldb/include/lldb/Interpreter/ScriptInterpreter.h
@@ -27,6 +27,7 @@
#include "lldb/Host/StreamFile.h"
#include "lldb/Interpreter/Interfaces/OperatingSystemInterface.h"
#include "lldb/Interpreter/Interfaces/ScriptedFrameInterface.h"
+#include "lldb/Interpreter/Interfaces/ScriptedFrameProviderInterface.h"
#include "lldb/Interpreter/Interfaces/ScriptedPlatformInterface.h"
#include "lldb/Interpreter/Interfaces/ScriptedProcessInterface.h"
#include "lldb/Interpreter/Interfaces/ScriptedThreadInterface.h"
@@ -536,6 +537,10 @@ class ScriptInterpreter : public PluginInterface {
return {};
}
+ virtual lldb::ScriptedFrameProviderInterfaceSP CreateScriptedFrameProviderInterface() {
+ return {};
+ }
+
virtual lldb::ScriptedThreadPlanInterfaceSP
CreateScriptedThreadPlanInterface() {
return {};
diff --git a/lldb/include/lldb/Interpreter/ScriptedFrameProvider.h b/lldb/include/lldb/Interpreter/ScriptedFrameProvider.h
new file mode 100644
index 0000000000000..6c4053f11eeb3
--- /dev/null
+++ b/lldb/include/lldb/Interpreter/ScriptedFrameProvider.h
@@ -0,0 +1,51 @@
+//===-- ScriptedFrameProvider.h --------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_INTERPRETER_SCRIPTEDFRAMEPROVIDER_H
+#define LLDB_INTERPRETER_SCRIPTEDFRAMEPROVIDER_H
+
+#include "lldb/Utility/ScriptedMetadata.h"
+#include "lldb/Utility/Status.h"
+#include "lldb/lldb-forward.h"
+#include "llvm/Support/Error.h"
+
+namespace lldb_private {
+
+class ScriptedFrameProvider {
+public:
+ /// Constructor that initializes the scripted frame provider.
+ ///
+ /// \param[in] thread_sp
+ /// The thread for which to provide scripted frames.
+ ///
+ /// \param[in] scripted_metadata
+ /// The metadata containing the class name and arguments for the
+ /// scripted frame provider.
+ ///
+ /// \param[out] error
+ /// Status object to report any errors during initialization.
+ ScriptedFrameProvider(lldb::ThreadSP thread_sp,
+ const ScriptedMetadata &scripted_metadata,
+ Status &error);
+ ~ScriptedFrameProvider();
+
+ /// Get the stack frames from the scripted frame provider.
+ ///
+ /// \return
+ /// An Expected containing the StackFrameListSP if successful,
+ /// otherwise an error describing what went wrong.
+ llvm::Expected<lldb::StackFrameListSP> GetStackFrames();
+
+private:
+ lldb::ThreadSP m_thread_sp;
+ lldb::ScriptedFrameProviderInterfaceSP m_interface_sp;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_INTERPRETER_SCRIPTEDFRAMEPROVIDER_H
diff --git a/lldb/include/lldb/Target/Thread.h b/lldb/include/lldb/Target/Thread.h
index 688c056da2633..d0c33f557b12b 100644
--- a/lldb/include/lldb/Target/Thread.h
+++ b/lldb/include/lldb/Target/Thread.h
@@ -1294,6 +1294,10 @@ class Thread : public std::enable_shared_from_this<Thread>,
/// The PC value before execution was resumed. May not be available;
/// an empty std::optional is returned in that case.
std::optional<lldb::addr_t> GetPreviousFrameZeroPC();
+
+ void SetScriptedFrameProvider(const ScriptedMetadata &scripted_metadata);
+
+ void ClearScriptedFrameProvider();
protected:
friend class ThreadPlan;
@@ -1338,6 +1342,8 @@ class Thread : public std::enable_shared_from_this<Thread>,
lldb::StackFrameListSP GetStackFrameList();
+ llvm::Expected<lldb::StackFrameListSP> GetScriptedFrameList();
+
void SetTemporaryResumeState(lldb::StateType new_state) {
m_temporary_resume_state = new_state;
}
@@ -1400,6 +1406,9 @@ class Thread : public std::enable_shared_from_this<Thread>,
/// The Thread backed by this thread, if any.
lldb::ThreadWP m_backed_thread;
+ /// The Scripted Frame Provider, if any.
+ lldb::ScriptedFrameProviderSP m_frame_provider_sp;
+
private:
bool m_extended_info_fetched; // Have we tried to retrieve the m_extended_info
// for this thread?
diff --git a/lldb/include/lldb/lldb-forward.h b/lldb/include/lldb/lldb-forward.h
index af5656b3dcad1..85045a803b07a 100644
--- a/lldb/include/lldb/lldb-forward.h
+++ b/lldb/include/lldb/lldb-forward.h
@@ -188,6 +188,8 @@ class Scalar;
class ScriptInterpreter;
class ScriptInterpreterLocker;
class ScriptedFrameInterface;
+class ScriptedFrameProvider;
+class ScriptedFrameProviderInterface;
class ScriptedMetadata;
class ScriptedBreakpointInterface;
class ScriptedPlatformInterface;
@@ -411,6 +413,10 @@ typedef std::shared_ptr<lldb_private::ScriptSummaryFormat>
typedef std::shared_ptr<lldb_private::ScriptInterpreter> ScriptInterpreterSP;
typedef std::shared_ptr<lldb_private::ScriptedFrameInterface>
ScriptedFrameInterfaceSP;
+typedef std::shared_ptr<lldb_private::ScriptedFrameProvider>
+ ScriptedFrameProviderSP;
+typedef std::shared_ptr<lldb_private::ScriptedFrameProviderInterface>
+ ScriptedFrameProviderInterfaceSP;
typedef std::shared_ptr<lldb_private::ScriptedMetadata> ScriptedMetadataSP;
typedef std::unique_ptr<lldb_private::ScriptedPlatformInterface>
ScriptedPlatformInterfaceUP;
diff --git a/lldb/source/API/SBThread.cpp b/lldb/source/API/SBThread.cpp
index 4e4aa48bc9a2e..a18d540f2a017 100644
--- a/lldb/source/API/SBThread.cpp
+++ b/lldb/source/API/SBThread.cpp
@@ -39,6 +39,7 @@
#include "lldb/Target/ThreadPlanStepOut.h"
#include "lldb/Target/ThreadPlanStepRange.h"
#include "lldb/Utility/Instrumentation.h"
+#include "lldb/Utility/ScriptedMetadata.h"
#include "lldb/Utility/State.h"
#include "lldb/Utility/Stream.h"
#include "lldb/Utility/StructuredData.h"
@@ -1324,3 +1325,29 @@ SBValue SBThread::GetSiginfo() {
return SBValue();
return thread_sp->GetSiginfoValue();
}
+
+void SBThread::RegisterFrameProvider(const char *class_name,
+ SBStructuredData &dict) {
+ LLDB_INSTRUMENT_VA(this, class_name, args_data);
+
+ ThreadSP thread_sp = m_opaque_sp->GetThreadSP();
+ if (!thread_sp)
+ return;
+
+ if (!dict.IsValid() || !dict.m_impl_up)
+ return;
+
+ StructuredData::ObjectSP obj_sp = dict.m_impl_up->GetObjectSP();
+
+ if (!obj_sp)
+ return;
+
+ StructuredData::DictionarySP dict_sp =
+ std::make_shared<StructuredData::Dictionary>(obj_sp);
+ if (!dict_sp || dict_sp->GetType() == lldb::eStructuredDataTypeInvalid)
+ return;
+
+
+ ScriptedMetadata metadata(class_name, dict_sp);
+ thread_sp->SetScriptedFrameProvider(metadata);
+}
diff --git a/lldb/source/Commands/CommandObjectFrame.cpp b/lldb/source/Commands/CommandObjectFrame.cpp
index 88a02dce35b9d..02d62aa9249d1 100644
--- a/lldb/source/Commands/CommandObjectFrame.cpp
+++ b/lldb/source/Commands/CommandObjectFrame.cpp
@@ -16,6 +16,7 @@
#include "lldb/Interpreter/CommandReturnObject.h"
#include "lldb/Interpreter/OptionArgParser.h"
#include "lldb/Interpreter/OptionGroupFormat.h"
+#include "lldb/Interpreter/OptionGroupPythonClassWithDict.h"
#include "lldb/Interpreter/OptionGroupValueObjectDisplay.h"
#include "lldb/Interpreter/OptionGroupVariable.h"
#include "lldb/Interpreter/Options.h"
@@ -29,6 +30,7 @@
#include "lldb/Target/Target.h"
#include "lldb/Target/Thread.h"
#include "lldb/Utility/Args.h"
+#include "lldb/Utility/ScriptedMetadata.h"
#include "lldb/ValueObject/ValueObject.h"
#include <memory>
@@ -1223,6 +1225,97 @@ class CommandObjectFrameRecognizer : public CommandObjectMultiword {
~CommandObjectFrameRecognizer() override = default;
};
+#pragma mark CommandObjectFrameProvider
+
+#define LLDB_OPTIONS_frame_provider_register
+#include "CommandOptions.inc"
+
+class CommandObjectFrameProviderRegister : public CommandObjectParsed {
+public:
+ CommandObjectFrameProviderRegister(CommandInterpreter &interpreter)
+ : CommandObjectParsed(
+ interpreter, "frame provider register",
+ "Register frame provider into current thread.",
+ nullptr, eCommandRequiresThread),
+
+ m_class_options("frame provider", true, 'C', 'k', 'v', 0) {
+ m_all_options.Append(&m_class_options, LLDB_OPT_SET_1 | LLDB_OPT_SET_2,
+ LLDB_OPT_SET_ALL);
+ m_all_options.Finalize();
+
+ AddSimpleArgumentList(eArgTypeRunArgs, eArgRepeatOptional);
+ }
+
+ ~CommandObjectFrameProviderRegister() override = default;
+
+ Options *GetOptions() override { return &m_all_options; }
+
+ std::optional<std::string> GetRepeatCommand(Args ¤t_command_args,
+ uint32_t index) override {
+ // No repeat for "process launch"...
+ return std::string("");
+ }
+
+protected:
+ void DoExecute(Args &launch_args, CommandReturnObject &result) override {
+ ScriptedMetadata metadata(m_class_options.GetName(), m_class_options.GetStructuredData());
+
+ Thread *thread = m_exe_ctx.GetThreadPtr();
+ if (!thread) {
+ result.AppendError("invalid thread");
+ return;
+ }
+
+ thread->SetScriptedFrameProvider(metadata);
+ result.SetStatus(eReturnStatusSuccessFinishResult);
+ result.AppendMessageWithFormat(
+ "Successfully registered scripted frame provider '%s'\n",
+ m_class_options.GetName().c_str());
+ }
+
+ OptionGroupPythonClassWithDict m_class_options;
+ OptionGroupOptions m_all_options;
+};
+
+class CommandObjectFrameProviderClear : public CommandObjectParsed {
+public:
+ CommandObjectFrameProviderClear(CommandInterpreter &interpreter)
+ : CommandObjectParsed(interpreter, "frame provider clear",
+ "Delete registered frame provider.", nullptr) {}
+
+ ~CommandObjectFrameProviderClear() override = default;
+
+protected:
+ void DoExecute(Args &command, CommandReturnObject &result) override {
+ Thread *thread = m_exe_ctx.GetThreadPtr();
+ if (!thread) {
+ result.AppendError("invalid thread");
+ return;
+ }
+
+ thread->ClearScriptedFrameProvider();
+
+ result.SetStatus(eReturnStatusSuccessFinishResult);
+ }
+};
+
+class CommandObjectFrameProvider : public CommandObjectMultiword {
+public:
+ CommandObjectFrameProvider(CommandInterpreter &interpreter)
+ : CommandObjectMultiword(
+ interpreter, "frame provider",
+ "Commands for registering and viewing frame providers.",
+ "frame provider [<sub-command-options>] ") {
+ LoadSubCommand("register", CommandObjectSP(new CommandObjectFrameProviderRegister(
+ interpreter)));
+ LoadSubCommand(
+ "clear",
+ CommandObjectSP(new CommandObjectFrameProviderClear(interpreter)));
+ }
+
+ ~CommandObjectFrameProvider() override = default;
+};
+
#pragma mark CommandObjectMultiwordFrame
// CommandObjectMultiwordFrame
@@ -1243,6 +1336,8 @@ CommandObjectMultiwordFrame::CommandObjectMultiwordFrame(
LoadSubCommand("variable",
CommandObjectSP(new CommandObjectFrameVariable(interpreter)));
#if LLDB_ENABLE_PYTHON
+ LoadSubCommand("provider", CommandObjectSP(new CommandObjectFrameProvider(
+ interpreter)));
LoadSubCommand("recognizer", CommandObjectSP(new CommandObjectFrameRecognizer(
interpreter)));
#endif
diff --git a/lldb/source/Commands/CommandObjectThread.cpp b/lldb/source/Commands/CommandObjectThread.cpp
index bbec714642ec9..0092151a13dd8 100644
--- a/lldb/source/Commands/CommandObjectThread.cpp
+++ b/lldb/source/Commands/CommandObjectThread.cpp
@@ -35,6 +35,7 @@
#include "lldb/Target/ThreadPlanStepInRange.h"
#include "lldb/Target/Trace.h"
#include "lldb/Target/TraceDumper.h"
+#include "lldb/Utility/ScriptedMetadata.h"
#include "lldb/Utility/State.h"
#include "lldb/ValueObject/ValueObject.h"
diff --git a/lldb/source/Interpreter/CMakeLists.txt b/lldb/source/Interpreter/CMakeLists.txt
index 8af7373702c38..ab877ddeecba8 100644
--- a/lldb/source/Interpreter/CMakeLists.txt
+++ b/lldb/source/Interpreter/CMakeLists.txt
@@ -53,6 +53,7 @@ add_lldb_library(lldbInterpreter NO_PLUGIN_DEPENDENCIES
OptionGroupWatchpoint.cpp
Options.cpp
Property.cpp
+ ScriptedFrameProvider.cpp
ScriptInterpreter.cpp
ADDITIONAL_HEADER_DIRS
diff --git a/lldb/source/Interpreter/ScriptedFrameProvider.cpp b/lldb/source/Interpreter/ScriptedFrameProvider.cpp
new file mode 100644
index 0000000000000..b35ed5d20f6ba
--- /dev/null
+++ b/lldb/source/Interpreter/ScriptedFrameProvider.cpp
@@ -0,0 +1,86 @@
+//===-- ScriptedFrameProvider.cpp ----------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Interpreter/ScriptedFrameProvider.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Interpreter/Interfaces/ScriptedFrameProviderInterface.h"
+#include "lldb/Interpreter/ScriptInterpreter.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/Thread.h"
+#include "lldb/Utility/ScriptedMetadata.h"
+#include "lldb/Utility/Status.h"
+#include "llvm/Support/Error.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+ScriptedFrameProvider::ScriptedFrameProvider(
+ ThreadSP thread_sp, const ScriptedMetadata &scripted_metadata,
+ Status &error)
+ : m_thread_sp(thread_sp), m_interface_sp(nullptr) {
+ if (!m_thread_sp) {
+ error = Status::FromErrorString(
+ "cannot create scripted frame provider: Invalid thread");
+ return;
+ }
+
+ ProcessSP process_sp = m_thread_sp->GetProcess();
+ if (!process_sp) {
+ error = Status::FromErrorString(
+ "cannot create scripted frame provider: Invalid process");
+ return;
+ }
+
+ ScriptInterpreter *script_interp =
+ process_sp->GetTarget().GetDebugger().GetScriptInterpreter();
+ if (!script_interp) {
+ error = Status::FromErrorString("cannot create scripted frame provider: No "
+ "script interpreter installed");
+ return;
+ }
+
+ m_interface_sp = script_interp->CreateScriptedFrameProviderInterface();
+ if (!m_interface_sp) {
+ error = Status::FromErrorString(
+ "cannot create scripted frame provider: Script interpreter couldn't "
+ "create Scripted Frame Provider Interface");
+ return;
+ }
+
+ auto obj_or_err = m_interface_sp->CreatePluginObject(
+ scripted_metadata.GetClassName(), m_thread_sp,
+ scripted_metadata.GetArgsSP());
+ if (!obj_or_err) {
+ error = Status::FromError(obj_or_err.takeError());
+ return;
+ }
+
+ StructuredData::ObjectSP object_sp = *obj_or_err;
+ if (!object_sp || !object_sp->IsValid()) {
+ error = Status::FromErrorString(
+ "cannot create scripted frame provider: Failed to create valid script "
+ "object");
+ return;
+ }
+
+ error.Clear();
+}
+
+ScriptedFrameProvider::~ScriptedFrameProvider() = default;
+
+llvm::Expected<StackFrameListSP> ScriptedFrameProvider::GetStackFrames() {
+ if (!m_interface_sp)
+ return llvm::createStringError(
+ "cannot get stack frames: Scripted frame provider not initialized");
+
+ auto frames = m_interface_sp->GetStackFrames();
+
+ // TODO: Convert StructuredData::ArraySP to StackFrameListSP
+ // This is a placeholder for now
+ return nullptr;
+}
\ No newline at end of file
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/CMakeLists.txt b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/CMakeLists.txt
index 09103573b89c5..50569cdefaafa 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/CMakeLists.txt
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/CMakeLists.txt
@@ -23,6 +23,7 @@ add_lldb_library(lldbPluginScriptInterpreterPythonInterfaces PLUGIN
OperatingSystemPythonInterface.cpp
ScriptInterpreterPythonInterfaces.cpp
ScriptedFramePythonInterface.cpp
+ ScriptedFrameProviderPythonInterface.cpp
ScriptedPlatformPythonInterface.cpp
ScriptedProcessPythonInterface.cpp
ScriptedPythonInterface.cpp
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptInterpreterPythonInterfaces.h b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptInterpreterPythonInterfaces.h
index 3814f46615078..0b9c7eb107bf5 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptInterpreterPythonInterfaces.h
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptInterpreterPythonInterfaces.h
@@ -18,6 +18,7 @@
#include "OperatingSystemPythonInterface.h"
#include "ScriptedBreakpointPythonInterface.h"
#include "ScriptedFramePythonInterface.h"
+#include "ScriptedFrameProviderPythonInterface.h"
#include "ScriptedPlatformPythonInterface.h"
#include "ScriptedProcessPythonInterface.h"
#include "ScriptedStopHookPythonInterface.h"
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedFrameProviderPythonInterface.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedFrameProviderPythonInterface.cpp
new file mode 100644
index 0000000000000..b9a659c44e6f1
--- /dev/null
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedFrameProviderPythonInterface.cpp
@@ -0,0 +1,57 @@
+//===-- ScriptedFrameProviderPythonInterface.cpp -------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Host/Config.h"
+#include "lldb/Target/Thread.h"
+#include "lldb/Utility/Log.h"
+#include "lldb/lldb-enumerations.h"
+
+#if LLDB_ENABLE_PYTHON
+
+// LLDB Python header must be included first
+#include "../lldb-python.h"
+
+#include "../SWIGPythonBridge.h"
+#include "../ScriptInterpreterPythonImpl.h"
+#include "ScriptedFrameProviderPythonInterface.h"
+#include <optional>
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::python;
+using Locker = ScriptInterpreterPythonImpl::Locker;
+
+ScriptedFrameProviderPythonInterface::Scripte...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
medismailben
force-pushed
the
scripted-frame-provider
branch
3 times, most recently
from
89e7ab6 to
9b9d08e
Compare
medismailben
changed the title
medismailben
force-pushed
the
scripted-frame-provider
branch
2 times, most recently
from
5fd3ebc to
70d72a2
Compare
There was a problem hiding this comment.
Being able to modify the frame list from a script would indeed be amazing - thanks for looking into this! 🙂
As it happens, I am currently looking into gdb's "frame filter" API, which serves a very similar use case - and I am wondering if we should take some inspiration from the gdb APIs here. Gdb's frame filter API differs from the API in this PR in the following points:
- simpler API: gdb's API only has a single function
filter(frame_iter) -> frame_iterfunction. (This PR currently introduces two functions:get_merge_strategyandget_stack_frames) - lazy stack unwinding: afaict, your patch leads to eager materialization of all stack frames. Given that stacks can be pretty deep, it would be preferable to only create those stack frames lazily, as the users inspects the stack. gdb achieves this by only advancing the returned iterator as needed
- more flexible merging strategies: in gdb, I can use all of Python's iterator support (generator expressions,
yield, ...) - global registration: in gdb, frame filters are registered globally for all threads. In this PR, the frame providers are registered for each thread individually
- multiple frame filters: gdb supports registering multiple frame filters at the same time. I didn't use that mechanism, yet, but afaict they are simply chained
The relevant pieces of gdb's documentation:
- Writing a frame filter - high-level introduction
- Frame filter API - corresponding API reference
- Frame decorators API - close equivalent to lldb's
ScriptedFrame - Frame filter management - end users can change enable, disable and reorder frame filters
My motivation for using frame filters in gdb are C++ couroutines. I want to add frames for asynchronous operations. For more background see LLVM's documentatin on Async Stack Traces, in particular the coro bt example. At least for my use case, gdb's design decisions (global registration, flexible merging strategy, ...) are a pretty good fit. But not sure, maybe your current design is a better fit for your particular use case? Which use case are you envisioning?
By the way: would discourse be better-suited for discussing the best approach here? Happy to copy my reply over to discourse, if you would start a thread
|
|
||
| // Add real frames after scripted frames (shifted indices) | ||
| for (uint32_t i = 0; i < num_real; i++) { | ||
| StackFrameSP real_frame = real_frames_sp->GetFrameAtIndex(i); |
There was a problem hiding this comment.
afaict, this materializes the complete backtrace?
There was a problem hiding this comment.
Only if the user picks the eScriptedFrameProviderMergeStrategyPrepend or eScriptedFrameProviderMergeStrategyAppend merge strategies which is necessary.
You probably picked that up by looking at the other files that the default option is eScriptedFrameProviderMergeStrategyReplace
There was a problem hiding this comment.
independent of the chosen merge strategy, the get_stackframes Python function always has to return all frames it wants to add.
E.g., for C++ coroutines I would hence have to walk the complete stack to see if any of the stack frames contains a local __coro_frame / __promise variable (the presence of those variables indicatse that additional artificial frames should be injected).
I guess the same would be true for the CPython stack frame provider? Or would that work fundamentally differently?
There was a problem hiding this comment.
I'm still working on the CPython Frame Provider but the idea is to iterate over all the frames in the current thread and save the the frame indices for frames coming from the CPython module and since the script would be implemented from python, I was going to use some inspect API to unwind the stack and replace the CPython frames by the python interpreter ones.
| uint32_t num_real = real_frames_sp->GetNumFrames(true); | ||
| uint32_t num_scripted = scripted_frames_sp->GetNumFrames(false); | ||
|
|
||
| switch (strategy) { |
There was a problem hiding this comment.
could we leave the merging up to the ScriptedFrameProvider? E.g., by having a get_frames(iterator) -> iterator method similar to gdb's frame filters?
There was a problem hiding this comment.
I don't think the ScriptedFrameProvider should be responsible for that since the main source of truth should be the Thread itself.
There was a problem hiding this comment.
main source of truth should be the Thread itself
Not sure I can follow. Can you elaborate?
There was a problem hiding this comment.
ScriptedFrameProvider is just another source of stack frames and hence just an implementation detail.
I don't think it should be in the business of handling the merge strategy and produce the final StackFrameList held by the thread. I believe the thread should asks all its frame providers for all the stack frames available and handle the stack frame list creation itself.
|
Hello! Thanks for your interest in this PR.
A non-goal for this PR and for lldb in general is copy what gdb or other debuggers do.
That sounds pretty exciting. I think ScriptedFrameProvider would be a great fit to support coroutines as long as the ABI doesn't spawn new threads for that. My current use case for this is to translate CPython frames into something that's more understandable, to hopefully improve debugging python / C++ interoperability in lldb.
|
This patch introduces a new scripting affordance: `ScriptedFrameProvider`.
This allows users to provide custom stack frames for real native threads,
augmenting or replacing the standard unwinding mechanism. This is useful
for:
- Providing frames for custom calling conventions or languages
- Reconstructing missing frames from crash dumps or core files
- Adding diagnostic or synthetic frames for debugging
The frame provider supports four merge strategies:
- Replace: Replace entire stack with scripted frames
- Prepend: Add scripted frames before real stack
- Append: Add scripted frames after real stack
- ReplaceByIndex: Replace specific frames by index
With this change, frames can be synthesized from different sources:
- Either from a dictionary containing a PC address and frame index
- Or by creating a ScriptedFrame python object for full control
To use it, first register the scripted frame provider then use existing
commands:
(lldb) frame provider register -C my_module.MyFrameProvider
or
(lldb) script thread.RegisterFrameProvider("my_module.MyFrameProvider", lldb.SBStructuredData())
then
(lldb) bt
See examples/python/templates/scripted_frame_provider.py for details.
Architecture changes:
- Moved ScriptedFrame from `Plugins` to `Interpreter` to avoid
layering violations
- Moved `RegisterContextMemory` from `Plugins` to `Target` as it only
depends on Target and Utility layers
- Added `ScriptedFrameProvider` C++ wrapper and Python interface
- Updated `Thread::GetStackFrameList` to apply merge strategies
rdar://161834688
Signed-off-by: Med Ismail Bennani <[email protected]>
While compatibility is not generally a goal, there's also no reason to diverge for the sake of it. However, what I think Adrian is suggesting here, is that we can benefit from building on top of the learnings of an existing implementation. I think he raises some good points that are worth evaluating in the context of LLDB. |
medismailben
force-pushed
the
scripted-frame-provider
branch
from
70d72a2 to
ab4a9b8
Compare
yes. Thanks for summing it up! Use cases
I polished my existing script and now posted it in PR #162145. The most interesting piece is The two lines prefixed with
Could you provide an example of a I am currently slightly struggling with imagining the intended usage of the proposed APIs, and such an example might help me. (Of course only if possible and not blocked, e.g., due to intellectual property issues) |
| # Attach a frame provider to a thread | ||
| thread = process.GetSelectedThread() | ||
| error = lldb.SBError() |
There was a problem hiding this comment.
Good catch, it's actually the return value of SBThread::SetScriptedFrameProvider. I'll update the doc.
This patch introduces a new scripting affordance:
ScriptedFrameProvider.This allows users to provide custom stack frames for real native threads,
augmenting or replacing the standard unwinding mechanism. This is useful
for:
The frame provider supports four merge strategies:
With this change, frames can be synthesized from different sources:
Architecture changes:
PluginstoInterpreterto avoidlayering violations
RegisterContextMemoryfromPluginstoTargetas it onlydepends on Target and Utility layers
ScriptedFrameProviderC++ wrapper and Python interfaceThread::GetStackFrameListto apply merge strategiesrdar://161834688
Signed-off-by: Med Ismail Bennani [email protected]